Skip to content

Conversation

@iamgabrielma
Copy link
Contributor

@iamgabrielma iamgabrielma commented Oct 29, 2025

Continuation of #16283
Closes WOOMOB-1570

Description

This PR adds the card reader connection modal flow through POS settings. For this we copy the existing implementation held in PointOfSaleDashboardView, except the onboarding part (this will come in the next PR), so the merchant already has to be onboarded.

I kept onboarding out of scope of the PR for now, so we can test it separately.

Changes

  • Renamed some of the affected files to use the POS prefix
  • POSSettingsHardwareDetailView contains 1 new modal, triggering PointOfSaleCardPresentPaymentAlert, which listens to cardPresentPaymentAlertViewModel and triggers the alert presentation for the card reader connection flow depending on the current reader state.

Test Steps

  • In POS > Settings, try the connection flow both starting disconnected, as well as connected. Connect, dismiss, move forward with a payment. It should work normally.
  • When disconnected, we show the "Connect card reader" card, while when connected we show the reader details. UI to be updated soon.
Screen.Recording.2025-10-29.at.17.18.11.480p.mov

@dangermattic
Copy link
Collaborator

dangermattic commented Oct 29, 2025

1 Warning
⚠️ This PR is assigned to the milestone 23.6. This milestone is due in less than 2 days.
Please make sure to get it merged by then or assign it to a milestone with a later deadline.

Generated by 🚫 Danger

@iamgabrielma iamgabrielma added type: task An internally driven task. feature: POS labels Oct 29, 2025
@iamgabrielma iamgabrielma added this to the 23.6 milestone Oct 29, 2025
@iamgabrielma iamgabrielma marked this pull request as ready for review October 29, 2025 10:47
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Oct 29, 2025

App Icon📲 You can test the changes from this Pull Request in WooCommerce iOS Prototype by scanning the QR code below to install the corresponding build.

App NameWooCommerce iOS Prototype
Build Numberpr16290-3b4822d
Version23.5
Bundle IDcom.automattic.alpha.woocommerce
Commit3b4822d
Installation URL16iq0ae5m8mcg
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@joshheald joshheald self-assigned this Oct 29, 2025
Copy link
Contributor

@joshheald joshheald left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works as expected. Some questions/suggestions inling

Comment on lines 345 to 352
private func handleScannerDestination(_ destination: ScannerDestination) {
switch destination {
case .setup:
showBarcodeScanningSetupModal = true
case .documentation:
showBarcodeScanningDocumentationModal = true
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be part of this PR? It seems to work fine but just be unrelated...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, is not new code, just moved it from lines 93-100 from being declared in the view to the private extension on 345-352. I see I forgot to remove the double private access control though, fixed here: 3b4822d

Comment on lines +85 to +90
.posModal(item: $posModel.cardPresentPaymentAlertViewModel, onDismiss: {
posModel.cardPresentPaymentAlertViewModel?.onDismiss?()
}, content: { alertType in
PointOfSaleCardPresentPaymentAlert(alertType: alertType)
.posInteractiveDismissDisabled(alertType.isDismissDisabled)
})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps there could be a shared definition of this somehow? Not if it's difficult; we're not going to use it in that many places, but it's essentially identical to PointOfSaleDashboardView's use.

I think it would have to be a view modifier, something like this:

struct CardReaderConnectingModifier: ViewModifier {
    @Environment(PointOfSaleAggregateModel.self) private var posModel

    func body(content: Content) -> some View {
        @Bindable var posModel = self.posModel
        content
            .posModal(item: $posModel.cardPresentPaymentAlertViewModel,
                      onDismiss: {
                posModel.cardPresentPaymentAlertViewModel?.onDismiss?()
            }) { alertType in
                PointOfSaleCardPresentPaymentAlert(alertType: alertType)
                    .posInteractiveDismissDisabled(alertType.isDismissDisabled)
            }
    }
}

extension View {
    func cardReaderConnecting() -> some View {
        modifier(CardReaderConnectingModifier())
    }
}

Though perhaps it would be better named as POS specific, and have the posModal requirement more obvious than using the environment var directly. It might be interesting to try some options for a few minutes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That definitely looks cleaner, thanks! I'll play with this separately and merge this PR for now.

@iamgabrielma iamgabrielma merged commit 0865397 into trunk Oct 30, 2025
13 checks passed
@iamgabrielma iamgabrielma deleted the task/WOOMOB-1570v2-pos-settings-reader-connection-modal-flow branch October 30, 2025 04:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature: POS type: task An internally driven task.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants